🦙 MegaLinter status: ⚠️ WARNING

See detailed report in MegaLinter reports

MegaLinter is graciously provided by

OK, so looking at the CausalTestCase class, it seems that the treatment_variable and outcome_variable properties aren't ever actually used (except in serialising to/from JSON), so we could get rid of them altogether and just store them in the associated Estimator. Programmatically this makes sense, but intuitively I'm not keen. The vibes are off - it puts too much responsibility on the "estimator". However, since the estimator also stores the adjustment set and (in the case of regression), the equational relationship, I think it might make sense to rename the Estimator family to something like CausalRelationship or something from the world of structural equational models. What are other people's thoughts?

For reference, instantiating a causal test case currently looks like this:

estimation_model = LinearRegressionEstimator(
            treatment_variable="A",
            outcome_variable="C",
            treatment_value=self.treatment_value,
            control_value=self.control_value,
            adjustment_set=self.minimal_adjustment_set,
        )
        causal_test_case = CausalTestCase(
            treatment_variable="A",
            outcome_variable="C",
            expected_causal_effect=self.expected_causal_effect,
            estimator=estimation_model,
            effect_measure="ate",
        )

In the new system, it'd look like this, which would be a lot cleaner:

causal_relationship = LinearRegressionRelationship(
            treatment_variable="A",
            outcome_variable="C",
            treatment_value=self.treatment_value,
            control_value=self.control_value,
            adjustment_set=self.minimal_adjustment_set,
        )
        causal_test_case = CausalTestCase(
            expected_causal_effect=self.expected_causal_effect,
            causal_relationship=causal_relationship,
            effect_measure="ate",
        )

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.54%. Comparing base (3e6fd99) to head (e753b1e).
⚠️ Report is 1 commits behind head on main.

@@            Coverage Diff             @@
##             main     #402      +/-   ##
==========================================
+ Coverage   98.18%   98.54%   +0.36%     
==========================================
  Files          32       25       -7     
  Lines        1704     1514     -190     
==========================================
- Hits         1673     1492     -181     
+ Misses         31       22       -9     

Update: Turns out removing treatment_variable and outcome_variable from CausalTestCase was really easy as nothing depended on it! I've added in treatment_variable and outcome_variable property methods for backwards compatibility and easy access to the variables, but the specification of test cases is a lot cleaner now. I think I will rename the Estimator classes to CausalRelationship to create a better intuition of what they do. This will essentially allow us to create full structural causal models!